CodSpeed Performance Report

Merging #4340 will not alter performance

_{Comparing onichandame:fix-20230 (adc7c31) with main (82dc77d)}

Summary

✅ 11 untouched benchmarks

Are you sure the test would have surfaced the previous error message that is mentioned in the issue this is supposed to solve?

@janpio According to my understanding of the code flow, the original error comes from the spot where the test covers. The traceroute goes:

1. https://github.com/prisma/prisma/blob/e5af35419069a4708c32fb3efef1c6f24f32e200/packages/migrate/src/commands/DbPush.ts#L92 where the db push command directly calls
2. https://github.com/prisma/prisma/blob/e5af35419069a4708c32fb3efef1c6f24f32e200/packages/migrate/src/utils/ensureDatabaseExists.ts#L162
3. https://github.com/prisma/prisma/blob/e5af35419069a4708c32fb3efef1c6f24f32e200/packages/internals/src/schemaEngineCommands.ts#L80 where it calls can-connect-to-database command, which is covered by the test mentioned in this PR

That being said, a direct reproduction from db push command is definitely more convincing. However I couldn't find a better place to insert the test for db push in prisma-engines

To me it looks like the error message that prisma/prisma#20230 talks about is based on this one, but additionally modified and expanded into weird grammar and wording. So I would not expect this PR to actually fix the problem, jsut to replace part of the message.

@janpio Sorry I missed out the grammar part of the problem. The wording of the error message is fixed. now the full message is

The provided database string is invalid. Expecting schemes postgres/postgresql/file/mysql/sqlserver/mongodb+srv/mongodb in database URL. Please refer to the documentation in https://www.prisma.io/docs/reference/database-reference/connection-urls for constructing a correct connection string. In some cases, certain characters must be escaped. Please check the string for any illegal characters.

I don't think that makes a difference. The issue has a lot more text before and after the error message you edited here, at least in the issue. If that is still valid, then your change will not fix that.

To make more concrete:

Error: P1013: The provided database string is invalid. <REDACTED> is not a known connection URL scheme. Prisma cannot determine the connector. in database URL. Please refer to the documentation in https://www.prisma.io/docs/reference/database-reference/connection-urls for constructing a correct connection string. In some cases, certain characters must be escaped. Please check the string for any illegal characters.

This is the problematic error message in the issue prisma/prisma#20230 This PR only modifies the bolded part of my quote above. I do not understand how the cut off sentence "in database URL" would be fixed by this PR.

@janpio I am not sure I understand your requirement. The text before and after it seem meaningful to me. Are you demanding to remove all the complementary text before and after the core text in the error message? Say the full error message: Expecting schemes postgres/postgresql/file/mysql/sqlserver/mongodb+srv/mongodb in database URL., is it what you want?

@janpio This is the final error message that goes to the end user's terminal. Could you kindly clarify which clauses need to be fixed?

What I did in this PR is simply changing the core part of the error message, so that the final error message makes sense when combined with the prefix and postfix

Ok, I understand now - the replaced error message changes the wording in a way that ending the message with in database URL. works, correct? Where does the in database URL. even come from?

We don't want to hardcode the list of expected schemes as postgres/postgresql/file/mysql/sqlserver/mongodb+srv/mongodb as that will certainly be forgotten when we add a new one. Can you formulate it more in the style of the old one?

@janpio Exactly. The in database URL... comes from

Please review again as the error message is rephrased to remove the hard-coded list of supported schemes. The full message is now:

The provided database string is invalid. The scheme is not recognized in database URL. Please refer to the documentation in https://www.prisma.io/docs/reference/database-reference/connection-urls for constructing a correct connection string. In some cases, certain characters must be escaped. Please check the string for any illegal characters.

Not the best grammar, but with the hardcoded "in database Url" is is hard to get something better.
Are there other error messages that use this as well and sound better?

@janpio there are quite a lot of places that have been using the formatter. The defining function has 19 references that calls it. One example is in postgres_setup. I don't have an example of the final error message yet. It seems that all the errors relating to url parsing are wrapped in the same format.

One way to fix the grammar is to remove the in database url part from the format, so that all the url parsing errors are free of the annoying postfix. But it may break some of the 19 callers. Another way is to make a new formatter for this specific error only, leaving all other callers unchanged, therefore less likely to break things. But it may bring more work in the future when we want to iterate the url parsing errors, because there will be 2 url parsing error formatters living side by side.

We may get more insights from the owner of this module. The original commit is authored by @tomhoule

There should be end to end tests for the messages. If there aren't, they could be written.

There should be end to end tests for the messages. If there aren't, they could be written.

cool. I can help with this. I imagine there are many diverse messages across the codebase that are not covered by test yet. To get started we can try to thoroughly test the url parsing error messages first. Is there a template e2e test that sets up the test in the way we like?